[fix][broker] Replication stuck when partitions count between two clusters is not the same #22983

poorbarcode · 2024-06-26T16:41:45Z

Motivation

Auto-creation rule of both clusters

cluster-c1: non-partitioned
cluster-c2: partitioned topic with 2 partitions

Issue 1 (Issue)

Enable double-way replication c1 -> c2 & c2 -> c1.
Create a non-partitioned topic tp1 on c1.
- The messages will be attempted to replicate into both partitions of c2, but it will fail due to a ClassCaseException^pic-1
At the moment, the topics on both clusters are below:
- non-partitioned topic tp-1
- partitioned topic tp-1 with 2 partitions.
The new partitions tp-1-partition-0 and tp-1-partition-1 on the cluster c2 will also start replication.
- Replicator will trigger topic auto-creation for c1
Finally, the topics on both clusters are below:
- c1:
  - non-partitioned topic tp-1
  - non-partitioned topic tp-1-partition-0
  - non-partitioned topic tp-1-partition-1
- c2: partitioned topic tp-1 with 2 partitions.
The new partitions tp-1-partition-0 and tp-1-partition-1 on the cluster c1 will also start replication.
- Since the replicator of non-partitioned topic tp-1 is already connected to the remote cluster, the new topics' replicator can not be created successfully and it will get an error Producer with name 'pulsar.repl.c1--> c2' is already connected to topic tp-1-partition-0/tp-1-partition-1

Issue 2

Enable one-way replication c1 -> c2.
Create a non-partitioned topic tp1 on c1.
- The messages will be attempted to replicate into both partitions of c2, but it will fail due to a ClassCaseException^pic-1
Finally, the topics on both clusters are below:
- c1: non-partitioned topic tp-1
- c2: partitioned topic tp-1 with 2 partitions.

pic-1:

Modifications

Replicator will only trigger a non-partitioned topic creation.

Documentation

doc
doc-required
doc-not-needed
doc-complete

Matching PR in forked repository

PR in forked repository: x

nodece · 2024-06-28T06:36:14Z

When a topic type is different(non-partitioned/partitioned) on the local and remote clusters, the pulsar-client has been thrown Producer with name 'pulsar.repl.c1--> c2' is already connected to topic tp-1-partition-0/tp-1-partition-1, I think the user has to handle this error manually, not depend on the pulsar to fix that.

codelipenghui · 2024-06-28T06:50:45Z

@poorbarcode You mentioned "break-change" in the PR's title. Do you mean this PR will introduce break change or it fixed a break change? If it will fix a break change, could you please link the change which introduced the break change?

poorbarcode · 2024-06-28T07:07:27Z

@codelipenghui

@poorbarcode You mentioned "break-change" in the PR's title. Do you mean this PR will introduce break change or it fixed a break change? If it will fix a break change, could you please link the change which introduced the break change?

Sorry, I assumed that the code class cast was added by #21946, if so, issue-2 described in Motivation can successfully copy data to c2 even though there are two partitions on the remote cluster.

I checked the history commits, and the class cast code was added when creating the class AbstractReplicator. I ran a test with the tag before #21946, and it does not work also.

So there is no break change( all the two scenarios can not work ), I removed the label break-change.

poorbarcode · 2024-06-28T07:10:28Z

@nodece

When a topic type is different(non-partitioned/partitioned) on the local and remote clusters, the pulsar-client has been thrown Producer with name 'pulsar.repl.c1--> c2' is already connected to topic tp-1-partition-0/tp-1-partition-1, I think the user has to handle this error manually, not depend on the pulsar to fix that.

Yes, it does. see here

if (metadata.partitions != 0) {
    log.error("[{}] The partitions count between two clusters is not the same(remote partitions: {}),"
                    + " the replicator can not be created successfully: {}", replicatorId, metadata.partitions,
            state);
    // This exception will be caught below, so it can be any typed.
    checkPartitionsSameFuture.completeExceptionally(new RuntimeException(replicatorId
            + "Can not replicate data to a partitioned topic."));
}

This PR only changes the scenario that there is no topic on the remote cluster. And makes the error logs clearer

poorbarcode · 2024-07-05T07:37:25Z

/pulsarbot rerun-failure-checks

pulsar-broker/src/test/java/org/apache/pulsar/broker/service/OneWayReplicatorTest.java

pulsar-client/src/main/java/org/apache/pulsar/client/impl/conf/ProducerConfigurationData.java

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractReplicator.java

… two clusters is not the same

codecov-commenter · 2024-07-15T15:00:29Z

Codecov Report

Attention: Patch coverage is 85.71429% with 4 lines in your changes missing coverage. Please review.

Project coverage is 73.45%. Comparing base (bbc6224) to head (ad95488).
Report is 449 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #22983      +/-   ##
============================================
- Coverage     73.57%   73.45%   -0.13%     
+ Complexity    32624     2412   -30212     
============================================
  Files          1877     1914      +37     
  Lines        139502   143616    +4114     
  Branches      15299    15668     +369     
============================================
+ Hits         102638   105489    +2851     
- Misses        28908    30049    +1141     
- Partials       7956     8078     +122

Flag	Coverage Δ
inttests	`27.52% <42.85%> (+2.93%)`	⬆️
systests	`24.69% <21.42%> (+0.36%)`	⬆️
unittests	`72.51% <85.71%> (-0.33%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...ache/pulsar/broker/service/AbstractReplicator.java	`65.35% <100.00%> (-19.65%)`	⬇️
...ar/client/impl/conf/ProducerConfigurationData.java	`87.20% <100.00%> (-4.56%)`	⬇️
...roker/service/persistent/PersistentReplicator.java	`67.98% <50.00%> (-0.89%)`	⬇️
...rg/apache/pulsar/client/impl/PulsarClientImpl.java	`74.52% <90.47%> (+0.22%)`	⬆️

... and 493 files with indirect coverage changes

…sters is not the same (#22983) (cherry picked from commit a8ce990)

poorbarcode · 2024-07-15T15:08:54Z

Since branch-3.2 does not contain PIP-344, so this PR can not cherry-pick into branch-3.2, removed the label release:3.2

…sters is not the same (#22983) (cherry picked from commit a8ce990)

…sters is not the same (apache#22983) (cherry picked from commit a8ce990) (cherry picked from commit 25542d8)

(cherry picked from commit fdd9747)

…sters is not the same (apache#22983) (cherry picked from commit a8ce990) (cherry picked from commit 25542d8)

(cherry picked from commit fdd9747)

poorbarcode added release/3.3.1 release/3.0.6 release/3.2.4 labels Jun 26, 2024

poorbarcode added this to the 3.4.0 milestone Jun 26, 2024

poorbarcode self-assigned this Jun 26, 2024

github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jun 26, 2024

poorbarcode changed the title ~~[improve][log] Improve replicator's log when partitions count between two clusters is not the same~~ [fix][broker] Replication stuck when partitions count between two clusters is not the same Jun 27, 2024

poorbarcode changed the title ~~[fix][broker] Replication stuck when partitions count between two clusters is not the same~~ [fix][broker] [break-change] Replication stuck when partitions count between two clusters is not the same Jun 27, 2024

poorbarcode requested review from lhotari, Technoboy-, codelipenghui, gaoran10, nodece and liudezhi2098 June 28, 2024 03:52

poorbarcode added type/bug The PR fixed a bug or issue reported a bug category/reliability The function does not work properly in certain specific environments or failures. e.g. data lost labels Jun 28, 2024

poorbarcode changed the title ~~[fix][broker] [break-change] Replication stuck when partitions count between two clusters is not the same~~ [fix][broker] Replication stuck when partitions count between two clusters is not the same Jun 28, 2024

poorbarcode added the ready-to-test label Jul 5, 2024

Demogorgon314 reviewed Jul 5, 2024

View reviewed changes

pulsar-broker/src/test/java/org/apache/pulsar/broker/service/OneWayReplicatorTest.java Outdated Show resolved Hide resolved

pulsar-client/src/main/java/org/apache/pulsar/client/impl/conf/ProducerConfigurationData.java Outdated Show resolved Hide resolved

nodece reviewed Jul 5, 2024

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractReplicator.java Outdated Show resolved Hide resolved

poorbarcode requested review from nodece and Demogorgon314 and removed request for nodece July 5, 2024 19:23

[improve][log] Improve replicator's log when partitions count between…

0eaf586

… two clusters is not the same

poorbarcode added 5 commits July 15, 2024 12:28

improve test

26d3ce5

fix test

d843f30

address comments

7c0d811

address comments

853904a

add more tests

1ceee52

poorbarcode force-pushed the improve/replicator_log branch from 7143776 to 1ceee52 Compare July 15, 2024 04:28

fix bug

814b678

Technoboy- approved these changes Jul 15, 2024

View reviewed changes

poorbarcode added 4 commits July 15, 2024 14:05

remove unnecessary changes

efccc69

remove unnecessary changes

7ec9ceb

fix log's bug

879c1b4

cleanup the resource that the test created

9c15498

nodece approved these changes Jul 15, 2024

View reviewed changes

poorbarcode added 2 commits July 15, 2024 17:36

fix test

6f20dc1

checkstyle

ad95488

Demogorgon314 approved these changes Jul 15, 2024

View reviewed changes

poorbarcode merged commit a8ce990 into apache:master Jul 15, 2024
51 checks passed

poorbarcode added a commit that referenced this pull request Jul 15, 2024

[fix][broker] Replication stuck when partitions count between two clu…

31bfc0d

…sters is not the same (#22983) (cherry picked from commit a8ce990)

poorbarcode added the cherry-picked/branch-3.3 label Jul 15, 2024

poorbarcode removed the release/3.2.4 label Jul 15, 2024

poorbarcode added a commit that referenced this pull request Jul 15, 2024

[fix][broker] Replication stuck when partitions count between two clu…

25542d8

…sters is not the same (#22983) (cherry picked from commit a8ce990)

poorbarcode added the cherry-picked/branch-3.0 label Jul 15, 2024

shibd added a commit that referenced this pull request Jul 16, 2024

fix code style introduce by #22983

fdd9747

nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 25, 2024

[fix][broker] Replication stuck when partitions count between two clu…

2ed5312

…sters is not the same (apache#22983) (cherry picked from commit a8ce990) (cherry picked from commit 25542d8)

nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 25, 2024

fix code style introduce by apache#22983

867ca1d

(cherry picked from commit fdd9747)

srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 26, 2024

[fix][broker] Replication stuck when partitions count between two clu…

326271a

…sters is not the same (apache#22983) (cherry picked from commit a8ce990) (cherry picked from commit 25542d8)

srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 26, 2024

fix code style introduce by apache#22983

b4acd58

(cherry picked from commit fdd9747)

poorbarcode mentioned this pull request Aug 7, 2024

[fix] [broker] Fix compatibility issues for PIP-344 #23136

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[fix][broker] Replication stuck when partitions count between two clusters is not the same #22983

[fix][broker] Replication stuck when partitions count between two clusters is not the same #22983

poorbarcode commented Jun 26, 2024 •

edited

Loading

nodece commented Jun 28, 2024

codelipenghui commented Jun 28, 2024

poorbarcode commented Jun 28, 2024 •

edited

Loading

poorbarcode commented Jun 28, 2024 •

edited

Loading

poorbarcode commented Jul 5, 2024

codecov-commenter commented Jul 15, 2024

poorbarcode commented Jul 15, 2024

[fix][broker] Replication stuck when partitions count between two clusters is not the same #22983

[fix][broker] Replication stuck when partitions count between two clusters is not the same #22983

Conversation

poorbarcode commented Jun 26, 2024 • edited Loading

Motivation

Modifications

Documentation

Matching PR in forked repository

nodece commented Jun 28, 2024

codelipenghui commented Jun 28, 2024

poorbarcode commented Jun 28, 2024 • edited Loading

poorbarcode commented Jun 28, 2024 • edited Loading

poorbarcode commented Jul 5, 2024

codecov-commenter commented Jul 15, 2024

Codecov Report

poorbarcode commented Jul 15, 2024

poorbarcode commented Jun 26, 2024 •

edited

Loading

poorbarcode commented Jun 28, 2024 •

edited

Loading

poorbarcode commented Jun 28, 2024 •

edited

Loading